Hi @manofstick, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

Not longer makes sense after #966

@manofstick @dsyme

Can you offer some guidance of what needs to happen for this PR?

Thanks

Kevin

Hiya @KevinRansom,

What's needed is for me to do the work to fix it up! @dsyme kindly reviewed it a couple of months back, and created a list of comments which I need to stitch in, but as time passing had moved me on to other projects (and I have been unable to convince my wife to send the children off to an orphanage) I've unfortunately haven't found the time to return to FSharp.Core land for a while.

Anyway, from memory, and it is a bit stetchy given a number of passing months, I think I had a bit of a concern with this fix, which was for recursive types (and I think they had to be genericly defined-recursive types [I think concrete types where compiler tail-called them properly]) but came at the expense of removing my eliminate-tail-call idiom in some cases, which then meant that the whole of the project was potentially dubious, as this meant that performance gains for > 64-bit structs was blown out of the water. Maybe I'm not remembering correctly though.

Hmmm... I'll have to revisit.

Look I'll try to find some time, but I'm pretty stretched (and currently sick!). Do you have any critical deadlines that are looming, so I can be inspired by the fear of missing out...

@manofstick No rush mate, I just wondered if it was still an active PR.

Kevin

@manofstick
If you want for this to make F# 4.1, I am afraid I am going to need an updated PR pretty soon.

@KevinRansom

The vacuum of life must be a Dyson as I have been unable to escape it's grip for the past while!

Anyway I definitely haven't abandoned the idea of this pull request, and I think if anything it is much more desirable now given the recent work around extending the ease of creation, as well as extended role, of value types within F# land.

So, not that I have given major background brain resources to it, but I was hoping that a better solution to the recursive types issue would pop into my head, but it hasn't. (I.e. there is a dualing battle between eliminating "tail" instructions for > 64bit value types (on 64-bit jit) and blowing the stack on generic types that are then recursively defined.) Possible solution to this is 1) create a calling convention for the 64-bit jit that is tail recursive friendly or 2) create a parallel set of comparison/equality interfaces for values types that pass the type by reference. Now I think 2 is actually a reasonable idea, but massively outside the scope of this work, so my brain has failed me.

Anyway, really it's more just a time thing though, as taking my girls to ballet and swimming lessons and the like is the trump card that keeps getting played from my deck.

But I like deadlines (it means something is done!), so I'll try to bunker down this weekend. But reality is is probably outside the 4.1 timeframe I'm guessing.

Regards,
@manofstick

It's all cool mate, and thanks for taking care of this.

Kevin

@KevinRansom @dsyme

OK; well still busy so haven't got around to fixing this yet, which, as mentioned in my previous ranting was because I really wasn't too excited by the existing solution that I was proposing, as it seemed a bit of a compromise with my original mission of providing a better solution for value types. (i.e. I was fixing boxing issues, but the > 64 bit tail issue was back, front and center....)

Anyway, some neuron finally decided to fire back with a plan which I think will finally fulfill my original vision. It adds a bit more complexity, which I'm not really happy with, as the scope now extends to modifying code generation, but I think to should neatly resolve everything (hahaha famous last words!)

OK; well I haven't gone about implementing any of this yet, as I thought I should run it by your guys first.

So the problem is that with recursively defined types, because I'm removing "tail", I leave myself open for a recursive spiral down to stack-overflow land. An obviously undesirable state. The solution proposed in this original "fix" was to short circuit my way into my code, restoring some "tail" instructions on the way and thus solving stack-overflow, but now causing some extreme slow downs in cases of > 64bit value types.

What I'm now proposing is providing a new interface something along the lines of:

type IStructuralEquatable<'a> = // naming is open for discussion, I'm just being lazy
    inherit IStructuralEquatable //maybe; but maybe not even desirable
    abstract Equals : other:'a -> comparer:IEqualityComparer<'a> -> depth:int -> bool
    abstract GetHashCode : comparer: IEqualityComparer<'a> -> depth:int -> int

Which would be my main point of entry into comparison, which would be implemented "tail" free. When called recursively on types it would increment the depth. After a relatively modest depth traversal (10? 20? 2?) it would fall back to the original comparisons methods (i.e. with boxing, tail and all.)

(With possibly also an a derived IEqualityComparer<'a> interface for the comparer with an additional property to determine if we're PER or ER...)

Potential issues with this I see are:

• Added complexity
• using types from previous compiler builds wouldn't implement this interface.

Anyway, I'm been hassled for the last 20 minutes to go to a playground, so I haven't proofread this particularly well, but hopefully it is in sufficient detail to get the gist of what I'm thinking of. I'm guessing this is a reasonable amount of work, and given my recent inability to get behind a keyword it could take a while, so if you think it is an unworkable solution, then let me know before I attempt to enter any serious coding effort.

Thanks!

manofstick.

Ha! Looks like I really should reanimate this at some stage, as looks like the underlying technique has been officially recognized and may make it's way into the compiler:

Classes for the Masses

@CaptainHayashi

Not that this PR is in a working state, but this PR (well #513 really) was an actually implementation of the underlying concepts that you are adding syntactic sugar around...

@manofstick @dsyme

Can you offer some guidance about what we should do with this PR?

Thank you

Kevin

@KevinRansom

Sigh. I really need a week or two solid to face this one again. Well I do ponder if it was an (uncredited!) influence in Classes for the Masses, and if it was then at least it has been useful for something!

I did say that this phoenix would rise again and I do hope that I do get inspired to do it, but I see little point of starting from this PRs, and so would probably be better off just forking from master again.

I say close it - its doubtful I'll find time within the next few months to get to it - but would be more than happy if someone else grabbed it and polished this mud into a dorodango.

@manofstick, @KevinRansom

Sorry for not replying earlier, in honesty I've fallen behind on the Classes for the Masses work since ending my MSFT internship. Am interested in theory in picking it back up and seeing it go forwards, but haven't had time recently to do so (or look at this or #513 in depth)…

@manofstick @MattWindsor91

Please feel free to reopen this PR ... if you decide to pick it back up again.

Thanks for doing this

Kevin